-
Notifications
You must be signed in to change notification settings - Fork 173
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
configury: enable autogen.sh from an embedded hwloc dist tarball #235
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Gilles Gouaillardet <gilles@rist.or.jp>
Refs open-mpi/ompi#3302 |
with this PR, it is pretty trivial to untar and add an embedded hwloc tarball into ompi and as a standalone package (a la libevent, pmix or romio) |
@jsquyres Can you answer above? |
@ggouaillardet How would you generate this "embedded" tarball? What's the avantage against bringing a normal full hwloc tarball, running autogen, and then running a script that removes unneeded directories? I am trying to simplify your commit. For instance, I don't like having the netloc script explictly listed. |
then the tarball can be untarred and renamed into the pro is from ompi, you can
without this PR, some steps do not complete successfully |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm trying to remember a reason why we felt compelled to use embedded m4 vs. just invoking hwloc's own configure
, and I'm failing to come up with a reason. It could well have simply been hysterical raisins...?
I.e., we probably did it for several prior embedded packages -- e.g., PLPA -- and therefore hwloc fell into the "well, this is the way we have always done things" mindset. And/or it could have been before our "invoke a component's configure" support got good...?
The more I think about it, the more I think the "don't invoke a sub-configure" methodology probably came from long, long ago in Open MPI (circa v1.0) where every component had its own standalone configure
script. It was incredibly modular, but it took forever to run (and remembers that computers+filesystems were much older/slower than today). We had a great configure transformation at one point to make all (normal) OMPI components have embedded m4 that could be slurped up into the main Open MPI configure
script. This dramatically sped up configure
's execution. Embedded projects (like PLPA) probably got swept up in that craze, and we've probably just maintained that position as our default ever since.
However, for standalone projects like hwloc, supporting embeddable m4 is actually fairly complicated. Perhaps hwloc 2.0 is a good time to audit the use and utility of all the embedding stuff in hwloc and see if it's really worth it any more. E.g., is it really a problem to include a few extra directories in the OMPI-embadded hwloc (even if OMPI doesn't use that stuff)? It certainly would be nice to maintain the configure
CLI options to disable the stuff that OMPI (and MPICH and ...) don't want/don't use, but the extra gyrations to make "special" embedded tarballs and/or have the ability to have embedded m4 may not be worth it any more.
@@ -0,0 +1,13 @@ | |||
#!/bin/sh | |||
|
|||
chmod u+w $2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor quibble: I usually like to assign $n
variables to names to make the code a bit more readable. For example:
top_srcdir=$1
distdir=$2
hwloc_version=$3
chmod u+w $distdir
# ...
@@ -0,0 +1,13 @@ | |||
#!/bin/sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should include the copyright header here.
# but sadly, automake *requires* it | ||
EOF | ||
done | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove trailing blank line.
#!/bin/sh | ||
|
||
chmod u+w $2 | ||
makefiles="$2/doc/Makefile.am $2/doc/examples/Makefile.am $2/doc/doxygen-config.cfg.in $2/utils/Makefile.am $2/utils/hwloc/Makefile.am $2/utils/lstopo/Makefile.am $2/utils/netloc/infiniband/Makefile.am $2/utils/netloc/draw/Makefile.am $2/utils/netloc/mpi/Makefile.am $2/tests/Makefile.am $2/tests/hwloc/Makefile.am $2/tests/hwloc/linux/Makefile.am $2/tests/hwloc/linux/allowed/Makefile.am $2/tests/hwloc/linux/gather/Makefile.am $2/tests/hwloc/x86/Makefile.am $2/tests/hwloc/xml/Makefile.am $2/tests/hwloc/ports/Makefile.am $2/tests/hwloc/rename/Makefile.am $2/tests/hwloc/linux/allowed/test-topology.sh.in $2/tests/hwloc/linux/gather/test-gather-topology.sh.in $2/tests/hwloc/linux/test-topology.sh.in $2/tests/hwloc/x86/test-topology.sh.in $2/tests/hwloc/xml/test-topology.sh.in $2/tests/hwloc/wrapper.sh.in $2/utils/hwloc/hwloc-compress-dir.in $2/utils/hwloc/hwloc-gather-topology.in $2/utils/hwloc/test-hwloc-annotate.sh.in $2/utils/hwloc/test-hwloc-calc.sh.in $2/utils/hwloc/test-hwloc-compress-dir.sh.in $2/utils/hwloc/test-hwloc-diffpatch.sh.in $2/utils/hwloc/test-hwloc-distrib.sh.in $2/utils/hwloc/test-hwloc-info.sh.in $2/utils/hwloc/test-fake-plugin.sh.in $2/utils/hwloc/test-hwloc-dump-hwdata/Makefile.am $2/utils/hwloc/test-hwloc-dump-hwdata/test-hwloc-dump-hwdata.sh.in $2/utils/lstopo/test-lstopo.sh.in $2/contrib/systemd/Makefile.am $2/contrib/misc/Makefile.am $2/tests/netloc/Makefile.am $2/tests/netloc/tests.sh.in $2/utils/lstopo/lstopo-windows.c" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to not hard-code this list of files? I ask because this list will inevitably get forgotten if new directories (i.e., generated files) are added over time.
@@ -0,0 +1,13 @@ | |||
#!/bin/sh | |||
|
|||
chmod u+w $2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, do you need to chmod u-w
on the $distdir
at the end of this script? I.e., do you need to un-do the chmod
changes that you did? I don't know if this has an effect on the generated tarball or not.
@ggouaillardet Your hunk about adding netloc_ib_gather_raw.in to EXTRA_DIST shouldn't be needed. This file shouldn't be required in embedded anyway, just like everything else under utils/. What I didn't get yet is why your procedure requires patches that the current procedure doesn't. We are creating dummy directories manually to make automake happy. But why is your automake also looking for Makefiles that the current procedure doesn't require? By the way the status of netloc vs embedding is unclear. OMPI won't use it anytime soon. You likely want to disable netloc everywhere for now. I agree with @jsquyres about not listing tons of Makefiles in the embedded distscript. I am fine with stopping the embedding. I don't know if anybody else is using it outside of OMPI, I'll ping the devel list. |
i will make the requested changes sometimes this week let me emphasize i claim no insightful knowledge on how autotools work.
|
"I am not able to have both hwloc 1.11.6 and hwloc 2.x," Did you try with 2 different components or a single one? |
what i meant is i cannot embedded two components in the ompi source tree (e.g. hwloc1116 and hwloc2x) hwloc framework has only one static component, so only one is built and there is no need |
Yes, I think we've had that limitation in OMPI for quite a while (you can't have multiple hwloc components simultaneously in the tree -- there's esoteric m4 reasons why, and I don't remember the details at the moment). So moving to a "invoke the But remember -- this does come at a cost (which we may or may not care about): the extra files and space used in git cloning. It would probably be a good idea to look at the resulting size of the ompi source trees (and tarballs) with and without all the extra hwloc files (e.g., the hwloc bz2 tarball is ~4MB, expands out to 16MB, of which the PDF docs alone are about 1.7MB). |
The size can be easily reduced to 1.5MB tarball and 8MB expanded (by removing docs PDF, html and man, while keeping the doxygen tag, and by removing tarballs and xmls in tests/linux and tests/xml). You can still autoreconf, configure and build from the root after that. But we cannot go much lower than that without a careful look at makefiles and configure. Note that there's still another solution: stop including hwloc in OMPI and use the system-provided hwloc. It seems to me that hwloc is very often installed on clusters nowadays. Versions are somehow old, but does it really matter for OMPI? |
We could try generating the explicit list of dummy makefiles by grepping for AC_CONFIG_FILES and AC_CONFIG_LINKS in configure and config/*.m4 There might be a way to add a regression test just like the current tests/hwloc/embedded By the way, lstopo-windows.c is only needed because of AC_CONFIG_LINKS. So links are only used for non-Linux testing on Linux. We could manually symlink on Linux to simplify this. |
We've discussed exactly this idea (not embedding hwloc in Open MPI any more) recently. We all agree that this is the Right long-term solution. However, there still appear to be some barriers in the short term:
All of these things, taken together, made it a bit too scary for us to remove the embedded hwloc from Open MPI -- at least for now. |
Note to open pull requests: some things changed in the CI yesterday, you'll need to rebase on top of master to avoid total CI failure. |
Signed-off-by: Gilles Gouaillardet gilles@rist.or.jp